Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Do not suggest using a promise as timeout #13050

Closed
wants to merge 1 commit into from
Closed

Do not suggest using a promise as timeout #13050

wants to merge 1 commit into from

Conversation

darabos
Copy link

@darabos darabos commented Oct 8, 2015

Promises don't work. (#9332) I wish they did, but if they don't at least do not explicitly document that they do.

Review on Reviewable

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

gkalpak added a commit to gkalpak/angular.js that referenced this pull request Oct 9, 2015
Previously, it was not possible to use a promise as value for the
`timeout` property of an actions `config`, because that value would be
copied before being passed to `$http`.
This commit introduces a special value for the `timeout`, namely
`'promise'`. Setting action's `timeout` configuration property to
`'promise'`, will create a new promise and pass it to `$http` for each
request. The associated deferred, is attached to the resource instance,
under `$timeoutDeferred`.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  query: {
    method: 'GET',
    isArray: true,
    timeout: 'promise'
  }
});

var posts = Post.query();   // GET /posts
...
// Later we decide to cancel the request
// in order to make a new one
posts.$timeoutDeferred.resolve();
posts.query({author: 'me'});   // GET /posts?author=me
...
```

Fixes angular#9332
Closes angular#13050
gkalpak added a commit to gkalpak/angular.js that referenced this pull request Oct 9, 2015
Previously, it was not possible to use a promise as value for the
`timeout` property of an action's `config`, because that value would be
copied before being passed to `$http`.
This commit introduces a special value for the `timeout`, namely
`'promise'`. Setting an action's `timeout` configuration property to
`'promise'`, will create a new promise and pass it to `$http` for each
request. The associated deferred, is attached to the resource instance,
under `$timeoutDeferred`.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  query: {
    method: 'GET',
    isArray: true,
    timeout: 'promise'
  }
});

var posts = Post.query();   // GET /posts
...
// Later we decide to cancel the request
// in order to make a new one
posts.$timeoutDeferred.resolve();
posts.query({author: 'me'});   // GET /posts?author=me
...
```

Fixes angular#9332
Closes angular#13050
@darabos
Copy link
Author

darabos commented Oct 20, 2015

I signed it! We signed it as a corporation, Big Graph Kft.

But I'd much prefer #13058 as a solution to this problem!

gkalpak added a commit to gkalpak/angular.js that referenced this pull request Oct 30, 2015
Introduced changes:

- Deprecate passing a promise as `timeout` (for `$resource` actions).
  It never worked correctly anyway.
  Now a warnign is logged (using `$log.debug()`) and the property is
  removed.
- Add support for a boolean `cancellable` property to actions'
  configuration, the `$resource` classes `options` of the
  `$resourceProvider`'s defaults.
  If true, the `$cancelRequest` method (added to all returned values for
  non-instance calls) will abort the request (if it's not already
  completed or aborted).
  If there is `timeout` specified on the action's configuration, the value
  of `cancellable` is ignored.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  get: {
    method: 'GET',
    cancellable: true
  }
});

var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```

Fixes angular#9332
Closes angular#13050
Closes angular#13058
@petebacondarwin petebacondarwin added this to the 1.5.x - migration-facilitation milestone Nov 1, 2015
gkalpak added a commit to gkalpak/angular.js that referenced this pull request Nov 23, 2015
Introduced changes:

- Deprecate passing a promise as `timeout` (for `$resource` actions).
  It never worked correctly anyway.
  Now a warnign is logged (using `$log.debug()`) and the property is
  removed.
- Add support for a boolean `cancellable` property to actions'
  configuration, the `$resource` classes `options` of the
  `$resourceProvider`'s defaults.
  If true, the `$cancelRequest` method (added to all returned values for
  non-instance calls) will abort the request (if it's not already
  completed or aborted).
  If there is `timeout` specified on the action's configuration, the value
  of `cancellable` is ignored.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  get: {
    method: 'GET',
    cancellable: true
  }
});

var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```

Fixes angular#9332
Closes angular#13050
Closes angular#13058
gkalpak added a commit to gkalpak/angular.js that referenced this pull request Nov 23, 2015
Introduced changes:

- Deprecate passing a promise as `timeout` (for `$resource` actions).
  It never worked correctly anyway.
  Now a warning is logged (using `$log.debug()`) and the property is
  removed.
- Add support for a boolean `cancellable` property in actions'
  configuration, the `$resource` factory's `options` parameter and the
  `$resourceProvider`'s `defaults` property.
  If true, the `$cancelRequest` method (added to all returned values for
  non-instance calls) will abort the request (if it's not already
  completed or aborted).
  If there is a numeric `timeout` specified on the action's configuration,
  the value of `cancellable` will be ignored.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  get: {
    method: 'GET',
    cancellable: true
  }
});

var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```

BREAKING CHANGE:

Using a promise as `timeout` is no longer supported and will log a
warning. It never worked the way it was supposed to anyway.

Before:

```js
var deferred = $q.defer();
var User = $resource('/api/user/:id', {id: '@id'}, {
  get: {method: 'GET', timeout: deferred.promise}
});

var user = User.get({id: 1});   // sends a request
deferred.resolve();             // aborts the request

// Now, we need to re-define `User` passing a new promise as `timeout`
// or else all subsequent requests from `someAction` will be aborted
User = $resource(...);
user = User.get({id: 2});
```

After:

```js
var User = $resource('/api/user/:id', {id: '@id'}, {
  get: {method: 'GET', cancellable: true}
});

var user = User.get({id: 1});   // sends a request
instance.$cancelRequest();      // aborts the request

user = User.get({id: 2});
```

Fixes angular#9332
Closes angular#13050
Closes angular#13058
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 23, 2015
Introduced changes:

- Deprecate passing a promise as `timeout` (for `$resource` actions).
  It never worked correctly anyway.
  Now a warning is logged (using `$log.debug()`) and the property is
  removed.
- Add support for a boolean `cancellable` property in actions'
  configuration, the `$resource` factory's `options` parameter and the
  `$resourceProvider`'s `defaults` property.
  If true, the `$cancelRequest` method (added to all returned values for
  non-instance calls) will abort the request (if it's not already
  completed or aborted).
  If there is a numeric `timeout` specified on the action's configuration,
  the value of `cancellable` will be ignored.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  get: {
    method: 'GET',
    cancellable: true
  }
});

var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```

BREAKING CHANGE:

Using a promise as `timeout` is no longer supported and will log a
warning. It never worked the way it was supposed to anyway.

Before:

```js
var deferred = $q.defer();
var User = $resource('/api/user/:id', {id: '@id'}, {
  get: {method: 'GET', timeout: deferred.promise}
});

var user = User.get({id: 1});   // sends a request
deferred.resolve();             // aborts the request

// Now, we need to re-define `User` passing a new promise as `timeout`
// or else all subsequent requests from `someAction` will be aborted
User = $resource(...);
user = User.get({id: 2});
```

After:

```js
var User = $resource('/api/user/:id', {id: '@id'}, {
  get: {method: 'GET', cancellable: true}
});

var user = User.get({id: 1});   // sends a request
instance.$cancelRequest();      // aborts the request

user = User.get({id: 2});
```

Fixes angular#9332
Closes angular#13050
Closes angular#13058
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Nov 23, 2015
Introduced changes:

- Deprecate passing a promise as `timeout` (for `$resource` actions).
  It never worked correctly anyway.
  Now a warning is logged (using `$log.debug()`) and the property is
  removed.
- Provide a `cancelRequest` static method on the Resource that will abort
  the request (if it's not already completed or aborted).
  If there is a numeric `timeout` specified on the action's configuration,
  this method will have no effect.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  get: {
    method: 'GET'
  }
});

var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
Post.cancelRequest(currentPost);
currentPost = Post.get({id: 2});
...
```

BREAKING CHANGE:

Using a promise as `timeout` is no longer supported and will log a
warning. It never worked the way it was supposed to anyway.

Before:

```js
var deferred = $q.defer();
var User = $resource('/api/user/:id', {id: '@id'}, {
  get: {method: 'GET', timeout: deferred.promise}
});

var user = User.get({id: 1});   // sends a request
deferred.resolve();             // aborts the request

// Now, we need to re-define `User` passing a new promise as `timeout`
// or else all subsequent requests from `someAction` will be aborted
User = $resource(...);
user = User.get({id: 2});
```

After:

```js
var User = $resource('/api/user/:id', {id: '@id'}, {
  get: {method: 'GET'}
});

var user = User.get({id: 1});   // sends a request
User.cancelRequest(instance);   // aborts the request

user = User.get({id: 2});
```

Fixes angular#9332
Closes angular#13050
Closes angular#13058
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Nov 23, 2015
Introduced changes:

- Deprecate passing a promise as `timeout` (for `$resource` actions).
  It never worked correctly anyway.
  Now a warning is logged (using `$log.debug()`) and the property is
  removed.
- Provide a `cancelRequest` static method on the Resource that will abort
  the request (if it's not already completed or aborted).
  If there is a numeric `timeout` specified on the action's configuration,
  this method will have no effect.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  get: {
    method: 'GET'
  }
});

var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
Post.cancelRequest(currentPost);
currentPost = Post.get({id: 2});
...
```

BREAKING CHANGE:

Using a promise as `timeout` is no longer supported and will log a
warning. It never worked the way it was supposed to anyway.

Before:

```js
var deferred = $q.defer();
var User = $resource('/api/user/:id', {id: '@id'}, {
  get: {method: 'GET', timeout: deferred.promise}
});

var user = User.get({id: 1});   // sends a request
deferred.resolve();             // aborts the request

// Now, we need to re-define `User` passing a new promise as `timeout`
// or else all subsequent requests from `someAction` will be aborted
User = $resource(...);
user = User.get({id: 2});
```

After:

```js
var User = $resource('/api/user/:id', {id: '@id'}, {
  get: {method: 'GET'}
});

var user = User.get({id: 1});   // sends a request
User.cancelRequest(instance);   // aborts the request

user = User.get({id: 2});
```

Fixes angular#9332
Closes angular#13050
Closes angular#13058
@gkalpak gkalpak closed this in 98528be Nov 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants